Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve command option types #1356

Closed
wants to merge 2 commits into from
Closed

Improve command option types #1356

wants to merge 2 commits into from

Conversation

bschlenk
Copy link

@bschlenk bschlenk commented Sep 15, 2020

Pull Request

⚠️ For discussion only ⚠️

Use TypeScript's new template literal types to improve the command option types. This allows the option flags to be parsed at the type level, giving accurate types to the resulting program object instead of falling back on { [key: string]: any }.

Problem

Currently there is no real type checking for options, so I might access an option that doesn't exist.

Solution

TypeScript recently added template literal types, allowing types to match patterns in strings. This allows us to parse the option name, and whether it has a required/optional value. Note that this feature is not released yet, but could be released with TypeScript 4.1.0.

See a working example on the typescript playground. Scroll to the bottom and hover the program variable.

TypeScript will fail to compile if the option flags are not in the right format.

To get the right behavior, the result of chaining off of commander needs to be assigned to a new variable, since the type is built up as a result of the chaining.

ChangeLog

@shadowspawn
Copy link
Collaborator

Related: #1245

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 15, 2020

Very cool, thanks for sharing! There will be lots of challenges to making the inferred typing accurate. There is also:

Removing the option-keys from the program type when .storeOptionsAsProperties(false) would be a big win on its own, as there is currently no TypeScript assistance for detecting incorrect routine names because of the generic typing for the option-keys.

@shadowspawn shadowspawn marked this pull request as draft September 15, 2020 20:45
Wrap option in partial since it may be omitted. requiredOption is
guaranteed to get the value, so don't wrap it in partial.
@bschlenk
Copy link
Author

Another challenge is the --no- prefix. The way I'm doing this now, where the extracted type of each .option call is &ed with this ends up breaking if you have both --cheese and --no-cheese. It ends up doing { cheese?: string | true } & { cheese?: boolean }, which typescript merges as { cheese?: undefined }. We'd need to use something like this stackoverflow answer, but hopefully simpler.

@shadowspawn
Copy link
Collaborator

For interest, I did some quick research on whether possible to support multiple versions of TypeScript

@shadowspawn
Copy link
Collaborator

This PR is not active, but has demonstrated some pretty interesting possibilities.

It will be hard to do an accurate representation of the options due to the flexible ways they are described and added, and not easily cover all use cases (e.g. the typings will not be trivially available for parameter of an action handler). But deduced typings may be looked at again in future.

Thank you for your contributions.

@shadowspawn
Copy link
Collaborator

For some experimental strong inferred typing see: https://github.com/commander-js/extra-typings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants